Skip to content

Conversation

@jmmartinez
Copy link
Contributor

With +SPV_KHR_float_controls2 and when there is a non-int OpConstantNull we
would call MI.getOperand(1).getImm() when MI was not an OpTypeInt (the
associated test has an OpTypeArray zeroinitialized).
Under this conditions an assertion is triggered.

This patch adds the missing condition.

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2025

@llvm/pr-subscribers-backend-spir-v

Author: Juan Manuel Martinez Caamaño (jmmartinez)

Changes

With +SPV_KHR_float_controls2 and when there is a non-int OpConstantNull we
would call MI.getOperand(1).getImm() when MI was not an OpTypeInt (the
associated test has an OpTypeArray zeroinitialized).
Under this conditions an assertion is triggered.

This patch adds the missing condition.


Full diff: https://github.com/llvm/llvm-project/pull/166909.diff

2 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp (+13-11)
  • (added) llvm/test/CodeGen/SPIRV/non_int_constant_null.ll (+25)
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 0175f2fb3698b..b10846aeee6a4 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -612,13 +612,10 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
         // Collect the SPIRVTypes for fp16, fp32, and fp64 and the constant of
         // type int32 with 0 value to represent the FP Fast Math Mode.
         std::vector<const MachineInstr *> SPIRVFloatTypes;
-        const MachineInstr *ConstZero = nullptr;
+        const MachineInstr *ConstZeroInt32 = nullptr;
         for (const MachineInstr *MI :
              MAI->getMSInstrs(SPIRV::MB_TypeConstVars)) {
-          // Skip if the instruction is not OpTypeFloat or OpConstant.
           unsigned OpCode = MI->getOpcode();
-          if (OpCode != SPIRV::OpTypeFloat && OpCode != SPIRV::OpConstantNull)
-            continue;
 
           // Collect the SPIRV type if it's a float.
           if (OpCode == SPIRV::OpTypeFloat) {
@@ -629,14 +626,19 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
               continue;
             }
             SPIRVFloatTypes.push_back(MI);
-          } else {
+            continue;
+          }
+
+          if (OpCode == SPIRV::OpConstantNull) {
             // Check if the constant is int32, if not skip it.
             const MachineRegisterInfo &MRI = MI->getMF()->getRegInfo();
             MachineInstr *TypeMI = MRI.getVRegDef(MI->getOperand(1).getReg());
-            if (!TypeMI || TypeMI->getOperand(1).getImm() != 32)
-              continue;
-
-            ConstZero = MI;
+            bool IsInt32Ty = TypeMI &&
+                             TypeMI->getOpcode() == SPIRV::OpTypeInt &&
+                             TypeMI->getOperand(1).getImm() == 32;
+            if (IsInt32Ty)
+              ConstZeroInt32 = MI;
+            continue;
           }
         }
 
@@ -657,9 +659,9 @@ void SPIRVAsmPrinter::outputExecutionMode(const Module &M) {
           MCRegister TypeReg =
               MAI->getRegisterAlias(MF, MI->getOperand(0).getReg());
           Inst.addOperand(MCOperand::createReg(TypeReg));
-          assert(ConstZero && "There should be a constant zero.");
+          assert(ConstZeroInt32 && "There should be a constant zero.");
           MCRegister ConstReg = MAI->getRegisterAlias(
-              ConstZero->getMF(), ConstZero->getOperand(0).getReg());
+              ConstZeroInt32->getMF(), ConstZeroInt32->getOperand(0).getReg());
           Inst.addOperand(MCOperand::createReg(ConstReg));
           outputMCInst(Inst);
         }
diff --git a/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll b/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll
new file mode 100644
index 0000000000000..0ba016aaa30aa
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/non_int_constant_null.ll
@@ -0,0 +1,25 @@
+; RUN: llc -mtriple spirv64-unknown-unknown %s --spirv-ext=+SPV_KHR_float_controls2 -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -mtriple spirv64-unknown-unknown %s --spirv-ext=+SPV_KHR_float_controls2 -o - -filetype=obj | spirv-val %}
+
+@A = addrspace(1) constant [1 x i8] zeroinitializer
+
+; CHECK: OpName %[[#FOO:]] "foo"
+; CHECK: OpName %[[#A:]] "A"
+; CHECK: OpDecorate %[[#A]] Constant
+; CHECK: OpDecorate %[[#A]] LinkageAttributes "A" Export
+; CHECK: %[[#INT8:]] = OpTypeInt 8 0
+; CHECK: %[[#INT32:]] = OpTypeInt 32 0
+; CHECK: %[[#ONE:]] = OpConstant %[[#INT32]] 1
+; CHECK: %[[#ARR_INT8:]] = OpTypeArray %[[#INT8]] %7
+; CHECK: %[[#ARR_INT8_PTR:]] = OpTypePointer CrossWorkgroup %[[#ARR_INT8]]
+; CHECK: %[[#ARR_INT8_ZERO:]] = OpConstantNull %[[#ARR_INT8]]
+; CHECK: %13 = OpVariable %[[#ARR_INT8_PTR]] CrossWorkgroup %[[#ARR_INT8_ZERO]]
+; CHECK: %[[#FOO]] = OpFunction
+; CHECK: = OpLabel
+; CHECK: OpReturn
+; CHECK: OpFunctionEnd
+
+define spir_kernel void @foo() {
+entry:
+  ret void
+}

@jmmartinez
Copy link
Contributor Author

I've added a last commit where I refactor a redundant condition and rename ConstZero into ConstZeroInt32, but if it's not relevant I can remove it (it polutes the PR a bit and is an unrelated NFC).

Copy link
Contributor

@maarquitos14 maarquitos14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks for the fix. Just a nit, otherwise LGTM.

@jmmartinez jmmartinez force-pushed the users/jmmartinez/spirv/fix_assertion branch from db3552b to a38d0d1 Compare November 10, 2025 08:14
@jmmartinez jmmartinez enabled auto-merge (squash) November 10, 2025 08:21
@jmmartinez jmmartinez disabled auto-merge November 10, 2025 08:55
When +SPV_KHR_float_controls2 and there was a non-int OpConstantZero we
would call MI.getOperand(1).getImm() when MI was not an OpTypeInt (the
associated test has an OpTypeArray zeroinitialized).

This patch adds the missing condition.
@jmmartinez jmmartinez force-pushed the users/jmmartinez/spirv/fix_assertion branch from a38d0d1 to 1e86b00 Compare November 10, 2025 09:19
@jmmartinez jmmartinez merged commit 57dad86 into main Nov 10, 2025
8 of 10 checks passed
@jmmartinez jmmartinez deleted the users/jmmartinez/spirv/fix_assertion branch November 10, 2025 10:02
ckoparkar added a commit to ckoparkar/llvm-project that referenced this pull request Nov 10, 2025
* main: (1028 commits)
  [clang][DebugInfo] Attach `DISubprogram` to additional call variants (llvm#166202)
  [C2y] Claim nonconformance to WG14 N3348 (llvm#166966)
  [X86] 2012-01-10-UndefExceptionEdge.ll - regenerate test checks (llvm#167307)
  Remove unused standard headers: <string>, <optional>, <numeric>, <tuple> (llvm#167232)
  [DebugInfo] Add Verifier check for incorrectly-scoped retainedNodes (llvm#166855)
  [VPlan] Don't apply predication discount to non-originally-predicated blocks (llvm#160449)
  [libc++] Avoid overloaded `operator,` for (`T`, `Iter`) cases (llvm#161049)
  [tools][llc] Make save-stats.ll test target independent (llvm#167238)
  [AArch64] Fallback to PRFUM for PRFM with negative or unaligned offset (llvm#166756)
  [X86] ldexp-avx512.ll - add v8f16/v16f16/v32f16 test coverage for llvm#165694 (llvm#167294)
  [DropAssumes] Drop dereferenceable assumptions after vectorization. (llvm#166947)
  [VPlan] Simplify branch-cond with getVectorTripCount (llvm#155604)
  Remove unused <algorithm> inclusion (llvm#166942)
  [AArch64] Combine subtract with borrow to SBC. (llvm#165271)
  [AArch64][SVE] Avoid redundant extend of unsigned i8/i16 extracts. (llvm#165863)
  [SPIRV] Fix failing assertion in SPIRVAsmPrinter (llvm#166909)
  [libc++] Merge insert/emplace(const_iterator, Args...) implementations (llvm#166470)
  [libc++] Replace __libcpp_is_final with a variable template (llvm#167137)
  [gn build] Port 152bda7
  [libc++] Replace the last uses of __tuple_types with __type_list (llvm#167214)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants